Draft+3459+3463+hermes #3470
Closed
Draft+3459+3463+hermes #3470
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* bat/feat/list-and-offer-snapshots: rebasing tinies Added changelog
* fraccaman/limit-pgf-stewards: changelog: add #3442 cleanup bug fix: get correct storage key fix genesis files improve logs added maximum amount of stewards as genesis parameter ci: update antithesis workflow ci: update antithesis workflow
* grarco/sdk-query-height: Changelog #2891 Fallible trait bound for block height param in queries
* brent/parameterize-gas-scale: Updated example of expected string Fixes ibc e2e test fix unit test change comment on Gas Display fixes from comments changelog: add #3391 fix and clean up Light error handling remove hard-coded gas scale add gas scale to protocol params
* tomas/move-verify-shielded: changelog: add #3419 shielded_token: feature guard validation to avoid compilation into wasm move masp validation from SDK into shielded_token crate
* grarco/masp-fee-payment: Removes fallback logic when failed fee payment Renames misleading gas limit variable Removes useless write-log commit in fee payment Fixes typo Fixes masp amounts conversion Fixes broken docs Reuses token transfer Fixes typo Panics in fee payment if balance read fails Changelog #3393 Adds missing gas spending key arg to ibc tx Masp fee payment for shielded actions Fixes masp tx generation and integration tests Updates shielded wasm code to handle fee unshielding Removes unused denominate function Adds support for masp fee payment in sdk Refactors the write log api Different gas cost for storage deletes Removes write log precommit and leverages the batch log Adds integration tests for masp fee payment Refactors batch execution in case of masp fee payment Skips the execution of the first inner tx when masp fee payment Renames fee payment gas limit parameter Returns `BatchedTxResult` from masp fee payment `check_fees` drop the storage changes in case of failure `check_fees` checks masp fee payment Reworks masp fee payment to correctly handle errors. Misc refactors Passes the correct tx index to masp fee payment check Introduces masp fee payment
* grarco/early-sapling-balance-check: Extracts the sapling value balance directly in `validate_transparent_bundle` Changelog #2721 Early sapling balance check in masp vp
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3470 +/- ##
==========================================
- Coverage 53.92% 53.51% -0.42%
==========================================
Files 317 319 +2
Lines 107575 109669 +2094
==========================================
+ Hits 58011 58687 +676
- Misses 49564 50982 +1418 ☔ View full report in Codecov by Sentry. |
* yuji/ibc-e2e-config: longer interval add changelog clear on start change IBC e2e test config
yito88
reviewed
Jul 2, 2024
wasm/tx_ibc/src/lib.rs
Outdated
Comment on lines
18
to
38
| // Prepare the sources of the multi-transfer | ||
| let sources = transfers | ||
| .sources | ||
| .into_iter() | ||
| .map(|(account, amount)| { | ||
| ((account.owner, account.token), amount.amount()) | ||
| }) | ||
| .collect::<BTreeMap<_, _>>(); | ||
|
|
||
| // Prepare the target of the multi-transfer | ||
| let targets = transfers | ||
| .targets | ||
| .into_iter() | ||
| .map(|(account, amount)| { | ||
| ((account.owner, account.token), amount.amount()) | ||
| }) | ||
| .collect::<BTreeMap<_, _>>(); | ||
|
|
||
| // Effect the multi transfer | ||
| token::multi_transfer(ctx, &sources, &targets) | ||
| .wrap_err("Token transfer failed")?; |
There was a problem hiding this comment.
ah, I see. The transfer has been added for the fee payment in #3393. However, masp_fee_payment was discarded.
if let Some(UnshieldingTransferData {
token,
amount,
target,
}) = &masp_fee_payment
{
// Transparent unshield for fee payment
token::transfer(
ctx,
&Address::Internal(address::InternalAddress::Masp),
target,
token,
amount.amount(),
)?;
}There was a problem hiding this comment.
Yep, exactly. @grarco Let me know if you have any objections to this merge resolution. Essentially it removes the redundant Transfer (that isn't executed by tx_ibc) from MsgTransfer (and similar IBC messages).
There was a problem hiding this comment.
In general it looks ok to me, since the new Transfer object is vectorized we can embed the fee payment directly inside it. But the run_ledger_ibc_with_hermes test is failing and it contains a masp fee payment over ibc so it's better if we check it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Indicate on which release or other PRs this topic is based on
Checklist before merging to
draft